Conversation
…iration timestamp
openeo_driver/views.py
Outdated
| return None | ||
|
|
||
| def download_url(filename) -> str: | ||
| if os.getenv("SIGNED_URL", "FALSE").upper() == "TRUE": |
There was a problem hiding this comment.
use smart_bool (imported already here from openeo_driver.utils)
openeo_driver/views.py
Outdated
| return _compute_secure_token(job_id, user.user_id, filename, expires) | ||
|
|
||
| def expiration_timestamp() -> Union[str, None]: | ||
| if 'SIGNED_URL_EXPIRATION' in os.environ: |
There was a problem hiding this comment.
I think it's cleaner to use current_app.config instead of os.environ (see usage in other places of views.py)
this config is populated here:
openeo-python-driver/openeo_driver/server.py
Lines 19 to 23 in 5c1e927
at this place you could get the values from
os.environ. I would just avoid using os.environ in views.py directly
openeo_driver/views.py
Outdated
| def secure_token(filename, expires) -> str: | ||
| return _compute_secure_token(job_id, user.user_id, filename, expires) | ||
|
|
||
| def expiration_timestamp() -> Union[str, None]: |
There was a problem hiding this comment.
expiration timestamp return value can still be int at this point I think
openeo_driver/views.py
Outdated
|
|
||
|
|
||
| def _compute_secure_token(job_id, user_id, filename, expiration_timestamp): | ||
| token_key = job_id + user_id + filename + str(expiration_timestamp) + os.environ['SIGNED_URL_SECRET'] |
There was a problem hiding this comment.
see above: avoid os.environ here
tests/test_views.py
Outdated
| 'type': 'Feature' | ||
| } | ||
|
|
||
| @mock.patch.dict(os.environ, {'SIGNED_URL': 'TRUE', 'SIGNED_URL_SECRET': '123&@#'}) |
There was a problem hiding this comment.
regarding the avoiding of os.environ not of above: I think you can just do @mock.patch.dict(app.config, {'SIG... here (app object is already available at this point)
openeo_driver/errors.py
Outdated
| super().__init__(message=self.message.format(file=filename)) | ||
|
|
||
|
|
||
| class FileExpiredException(OpenEOApiException): |
There was a problem hiding this comment.
New errors in this file have first to be defined through https://github.com/Open-EO/openeo-api/blob/master/errors.json
There was a problem hiding this comment.
in the mean time, you can just raise a generic exception:
raise OpenEOApiException(
status_code=410,
code="FileExpired",
message="File '{file}' has expired".format(file=filename)"
)There was a problem hiding this comment.
Interesting suggestion from Open-EO/openeo-api#379 (comment):
raise it with something like:
raise OpenEOApiException(
status_code=410,
code="AssetLinkExpired",
message="Batch job asset link has expired. Request the batch job results again for fresh asset links" ,
)
|
FYI: Open-EO/openeo-api#380 is now merged in draft and the newly introduced error is so you could already adapt that error code |
|
is that bump of the openeo_driver/specs/openeo-api/0.4 submodule intentional? |
3 participants
No description provided.